-
Notifications
You must be signed in to change notification settings - Fork 104
Fix chemistry‑related bugs #829
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
has the code that automatically turns a 1D case into an extrapolated 2D one been put into MFC? @DimAdam-01 |
I consider it a distinct feature that I intended to add separately, but I can include it in this pull request. @sbryngelson |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #829 +/- ##
==========================================
+ Coverage 43.15% 43.34% +0.18%
==========================================
Files 68 68
Lines 20262 20128 -134
Branches 2424 2400 -24
==========================================
- Hits 8745 8725 -20
+ Misses 10054 9934 -120
- Partials 1463 1469 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
toolchain/pyproject.toml
Outdated
"cantera==3.0.1", | ||
"pyrometheus==1.0.2" | ||
"cantera==3.1.0", | ||
"pyrometheus@git+https://github.com/pyrometheus/pyrometheus.git@main" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DimAdam-01 I think it would be preferable to reference an official (versioned) release of Pyro instead of referencing the main branch. You can see the original code referenced the 1.0.2 release on Pypi
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DimAdam-01 i just created a new pyro release 1.0.3 (not sure if it's on pypi just yet)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just tested it, it's not on pypi yet: (ERROR: No matching distribution found for pyrometheus==1.0.3)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah i created it 3 min ago
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can change to pyrometheus==1.0.3
now
Description
resolve the issue that prevented multi‑rank execution when using non‑hardcoded initial conditions;
and fix minor bugs in both the inline Riemann solver and the HLL Riemann solver.
Type of change
Scope
How Has This Been Tested?
Run the 1D shocktube reaction case, compared the result and they had the exact match,
Test Configuration:
Run on Delta and Delta AI ( A100 and GH GPUs), as well as on my personal laptop
Checklist
docs/
)examples/
that demonstrate my new feature performing as expected.They run to completion and demonstrate "interesting physics"
./mfc.sh format
before committing my codeIf your code changes any code source files (anything in
src/simulation
)To make sure the code is performing as expected on GPU devices, I have:
nvtx
ranges so that they can be identified in profiles./mfc.sh run XXXX --gpu -t simulation --nsys
, and have attached the output file (.nsys-rep
) and plain text results to this PR./mfc.sh run XXXX --gpu -t simulation --omniperf
, and have attached the output file and plain text results to this PR.